Added a function to add backslash before a space #541
Conversation
|
If you do want to just replace all the In [3]: folder_name = 'foo bar baz'
In [4]: folder_name.replace(' ','\ ')
Out[4]: 'foo\\ bar\\ \\ baz'But there may be a better fix. This won't escape anything except spaces. Most functions don't need the path escaping. The few that do include
|
arc/common.py
Outdated
| arc_r_symbols = [atom.element.symbol for atom in | ||
| chain(*tuple(arc_reaction.r_species[i].mol.atoms for i in range(num_rs)))] | ||
| arc_p_symbols = [atom.element.symbol for atom in | ||
| chain(*tuple(arc_reaction.p_species[i].mol.atoms for i in range(num_ps)))] |
There was a problem hiding this comment.
All the modifications to the file up to this point are style changes. If you think they are necessary, please add them as a different commit. I would recommend removing them, the previous version is "OK", I think
arc/common.py
Outdated
| new_folder_name (str): modified folder name. | ||
|
|
||
| """ | ||
| folder_name.split(" ") |
There was a problem hiding this comment.
this line has no effect (we don't store the returned value)
arc/common.py
Outdated
| """ | ||
| folder_name.split(" ") | ||
| new_folder_name = "" | ||
| for i in folder_name.split(" "): |
There was a problem hiding this comment.
minor: I would recommend changing i to a different name, i is usually an integer iterator. maybe use split?
There was a problem hiding this comment.
You can do either folder_name.split() or folder_name.split(" "), they are the same (the first one is shorter or "cleaner")
arc/common.py
Outdated
| folder_name.split(" ") | ||
| new_folder_name = "" | ||
| for i in folder_name.split(" "): | ||
| new_folder_name += i + "\ " |
There was a problem hiding this comment.
If you'd like, you can achieve this in a single line instead of the loop. Try:
new_folder_name = '\ '.join(folder_name.split())
MichalKesl
changed the title
Codecov Report
@@ Coverage Diff @@
## main #541 +/- ##
==========================================
+ Coverage 73.18% 73.20% +0.01%
==========================================
Files 99 99
Lines 26522 26534 +12
Branches 5546 5546
==========================================
+ Hits 19409 19423 +14
+ Misses 5772 5771 -1
+ Partials 1341 1340 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Thanks @MichalKesl, looks good!
I added some comments, please take a look
arc/common_test.py
Outdated
| self.assertTrue(common.fill_in_the_blanks(ex1), "michalkfir") | ||
| self.assertTrue(common.fill_in_the_blanks(ex2), "michal\\ kfir") | ||
| self.assertTrue(common.fill_in_the_blanks(ex3), "mich\\ al\\ kfir") | ||
| self.assertTrue(common.fill_in_the_blanks(ex4), "michal\\ \\ kfir") |
There was a problem hiding this comment.
why do we have two slashes in the tests?
There was a problem hiding this comment.
If we do michal kfir.replace(" ", "\ ") the string changes to michal\\ kfir, because \ is an operator (like \n).
|
@MichalKesl, when you get a chance, can we finish this PR? |
MichalKesl
force-pushed
the
folder_rename
branch
from
d48f504
to
df9dff7
Compare
|
@MichalKesl is this PR relevant? |
If path has spaces the function will modify the name and add backslash before the space. a local test was preformed, the solution works.
fixes #535